Rich Text: Remove restoreContentAndSplit #7618
Conversation
aduth
added
[Feature] Rich Text
|
Funny enough, this will incidentally fix the issue described at #5095 (comment), which is still technically an issue for Some related discussion today in Slack with @tofumatt |
|
The end-to-end test failure is a legitimate bug where the caret is in the first paragraph wrongly when splitting in two from the middle: But in fact it's the same bug observed at #5095 (comment) and proposed to be fixed by #7620 . You can observe that it becomes fixed again if you cherry-pick 29bcba5 into your local branch. Thus, considering #7620 as a blocker for this. |
aduth
added
the
[Status] Blocked
| const beforeFragment = beforeRange.extractContents(); | ||
| const afterFragment = afterRange.extractContents(); | ||
| const beforeFragment = beforeRange.cloneContents(); | ||
| const afterFragment = afterRange.cloneContents(); |
There was a problem hiding this comment.
Could the afterFragment still be extracted, so we can avoid having to set it afterwards? I think this this happens in the block edit component. Or maybe I'm misunderstanding this.
There was a problem hiding this comment.
so we can avoid having to set it afterwards?
What do you mean by set?
There was a problem hiding this comment.
By means of setAttributes in the block edit function.
There was a problem hiding this comment.
Still not totally sure I understand, but we should want the explicit setAttributes and avoid leveraging any of these more implicit DOM mutations.
aduth
force-pushed
the
remove/rich-text-restore-content-split
branch
from
1085589 to
03edbac
Compare
aduth
force-pushed
the
remove/rich-text-restore-content-split
branch
from
03edbac to
f3d3852
Compare
aduth
force-pushed
the
remove/rich-text-restore-content-split
branch
from
f3d3852 to
7a61eb0
Compare
Avoid destroying original content incurred by extractContents
Call `onSplit` directly
aduth
force-pushed
the
remove/rich-text-restore-content-split
branch
from
7a61eb0 to
03ded61
Compare
|
Rebased and force pushed. Running locally, all tests / functionality appears to work as expected now. |
aduth
removed
the
[Status] Blocked
There was a problem hiding this comment.
I tested this locally, split a bunch of blocks, paragraphs, lists, headings, including paragraphs that had bold characters and such–split them inside and outside the bold characters.
All seemed to work well and I encountered no bugs.
Writing a few e2e tests for block splitting would be good, I dunno if we have any. Either way I suppose that's not just related to this PR 😄
🚢
|
|
There's quite a few split across |
Related: https://github.com/WordPress/gutenberg/pull/5100/files#r198953121
This pull request seeks to simplify the
RichTextcomponent, removing itsrestoreContentAndSplitfunction which had been used to preserve content through a block split. In my testing, I had found that this was not a behavior caused by TinyMCE, as indicated by the function's comment, but rather in our use ofRange#extractContentswhich moves (i.e. destroys) contents of a range. Updating this code to useRange#cloneContentsinstead avoids the need to re-set theRichTextcontent which, in turn, should be a boon to performance and avoid the need for this pass-through function.Testing instructions:
Verify that there are no regressions in block splitting. Particularly, ensure that a paragraphs contents are not destroyed after pressing Enter within.